add experimental flag on plugins to disable them by default #5985
Conversation
Overall package sizeSelf size: 9.69 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.7.0 | 35.02 MB | 35.02 MB | | @datadog/native-appsec | 10.0.1 | 20.3 MB | 20.3 MB | | @datadog/native-iast-taint-tracking | 4.0.0 | 11.72 MB | 11.73 MB | | @datadog/pprof | 5.9.0 | 9.77 MB | 10.14 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.5.3 | 2.95 MB | 5.6 MB | | @datadog/wasm-js-rewriter | 4.0.1 | 2.85 MB | 3.58 MB | | @datadog/native-metrics | 3.1.1 | 1.02 MB | 1.43 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | jsonpath-plus | 10.3.0 | 617.18 kB | 1.08 MB | | import-in-the-middle | 1.14.2 | 122.36 kB | 850.93 kB | | lru-cache | 10.4.3 | 804.3 kB | 804.3 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.1 | 109.9 kB | 109.9 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 7.0.5 | 63.38 kB | 63.38 kB | | istanbul-lib-coverage | 3.2.2 | 34.37 kB | 34.37 kB | | rfdc | 1.4.1 | 27.15 kB | 27.15 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | dc-polyfill | 0.1.9 | 25.11 kB | 25.11 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | shell-quote | 1.8.3 | 23.74 kB | 23.74 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | semifies | 1.0.0 | 15.84 kB | 15.84 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | mutexify | 1.4.0 | 5.71 kB | 8.74 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | module-details-from-path | 1.0.4 | 3.96 kB | 3.96 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5985 +/- ##
==========================================
- Coverage 82.81% 82.77% -0.05%
==========================================
Files 476 476
Lines 19658 19652 -6
==========================================
- Hits 16280 16267 -13
- Misses 3378 3385 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Datadog ReportBranch report: ✅ 0 Failed, 1257 Passed, 0 Skipped, 19m 13.67s Total Time |
rochdev
force-pushed
the
experimental-plugins
branch
from
2e3f1b8 to
712addc
Compare
There was a problem hiding this comment.
Maybe it's just me, but I find the PR description a little confusing, so I'm not sure exactly what it's supposed to do in details. E.g. as I wrote in one of my comments below, it seems like this PR is also deprecating a disabled property, though I'm not sure.
| @@ -25,20 +25,15 @@ const names = Object.keys(hooks) | |||
| const pathSepExpr = new RegExp(`\\${path.sep}`, 'g') | |||
|
|
|||
| const disabledInstrumentations = new Set( | |||
| DD_TRACE_DISABLED_INSTRUMENTATIONS?.split(',').map(name => normalizePluginEnvName(name, true)) ?? [] | |||
| DD_TRACE_DISABLED_INSTRUMENTATIONS?.split(',') ?? [] | |||
There was a problem hiding this comment.
No need for a fallback. These all result in an empty set: new Set(), new Set([]), new Set(undefined)
| DD_TRACE_DISABLED_INSTRUMENTATIONS?.split(',') ?? [] | |
| DD_TRACE_DISABLED_INSTRUMENTATIONS?.split(',') |
| @@ -75,10 +69,6 @@ for (const packageName of names) { | |||
| if (hook !== null && typeof hook === 'object') { | |||
| if (hook.serverless === false && isInServerlessEnvironment()) continue | |||
|
|
|||
| // some integrations are disabled by default, but can be enabled by setting | |||
| // the DD_TRACE_<INTEGRATION>_ENABLED environment variable to true | |||
| if (hook.disabled && !reenabledInstrumentations.has(normalizedPackageName)) continue | |||
There was a problem hiding this comment.
I've not been able to find any hooks that are disabled by default currently. Am I correct in assuming that this was just an option, but that we currently didn't use it?
Does this PR also remove support for the disabled property on hooks?
What does this PR do?
Add experimental flag on plugins to disable them by default.
Motivation
Right now only disabling the instrumentation is possible by default. However, this is problematic because users interact with plugins, not instrumentation, so they would only know the name for the plugins. Also, instrumentations do more than just back a single plugin, so when the goal is to disable just tracing for a library for example, it shouldn't also disable any other existing features already built on top of the same instrumentation. Doing this at the plugin level instead solves these issues and provides a better and more granular user experience that focuses only on the functionality.